Bump Discord.py to latest alpha and use BotBase from bot-core#2118
Conversation
ChrisLovering
force-pushed
the
bump-d.py-version
branch
3 times, most recently
from
7532128 to
c5ab091
Compare
MarkKoz
left a comment
MarkKoz
left a comment
There was a problem hiding this comment.
Not an extensive review but it's a start.
Xithrius
added
a: dependencies
ChrisLovering
force-pushed
the
bump-d.py-version
branch
3 times, most recently
from
3853986 to
b1b3569
Compare
ChrisLovering
changed the title
ChrisLovering
force-pushed
the
bump-d.py-version
branch
2 times, most recently
from
e7ecabb to
501af18
Compare
ChrisLovering
requested review from
Den4200,
jb3,
ks129,
mbaruh and
wookie184
as code owners
ChrisLovering
added
s: needs review
ChrisLovering
force-pushed
the
bump-d.py-version
branch
from
501af18 to
8b4c942
Compare
|
Rebased onto main. Made resources cog use async setup too, since it was added after this PR was opened. |
ChrisLovering
added
p: 1 - high
|
Hi Chris, Thank you for your tremendous efforts in getting the bot up to date. I am sure the rest of the team is filled with excitement at the prospect of using all the new features and maintaining a slimmer code base thanks to your work on bot-core. I went through every commit and every line. Looks like everything is order. Notwithstanding, there are many changes, and it is easy for my eyes to miss something. Unfortunately, I did not run any of the code, nor do I really have time to do so and extensively test any of it. Would you still like me to leave an approval? |
If you're happy with the code quality, I'd appreciate the approval. I think as for regression testing (since there should be a lot) I'll post something in the server and see if we can crowd source it. |
ChrisLovering
force-pushed
the
bump-d.py-version
branch
from
04ca07b to
88088e6
Compare
wookie184
left a comment
wookie184
left a comment
There was a problem hiding this comment.
Great PR! Just some small things. I haven't tested this, but noticed it was being tested on the test server so should be fine. Might try and play around with it if I get a chance.
This is to remove possible confusion with time.time
wookie184
left a comment
wookie184
left a comment
There was a problem hiding this comment.
Changes all look good - managed to give it a quick test run and everything I tried worked nicely! Just one thing I noticed.
Xithrius
removed
the
s: needs review
4 participants
This is quite a large PR due to the breaking changes from Discord.py. Most notably there was a large change around asyncio, which you can read here.
With cogs being changed to have async cog_load (and unload) functions, I have updated all the places that previously used the init_task pattern to instead now use this async cog_load function.
This also impacted how the bot is initialised and loaded, since you cannot use wait_for or similar within these async functions. As such, we created BotBase which abstracts this, and many other things, away from you.
These drastic changes also meant quite a few changes to the tests needed to be made.
I have tried to keep the commits fairly atomic, so review commit-by-commit may be best.